Skip to content

security: fail-closed on revoked/out-of-policy memory activity and propagate revocation load errors - #85

Merged
BunsDev merged 2 commits into
mainfrom
codex/propose-fix-for-memory-revocation-vulnerability
Aug 26, 2026
Merged

security: fail-closed on revoked/out-of-policy memory activity and propagate revocation load errors#85
BunsDev merged 2 commits into
mainfrom
codex/propose-fix-for-memory-revocation-vulnerability

Conversation

@BunsDev

@BunsDev BunsDev commented Aug 26, 2026

Copy link
Copy Markdown
Member

Motivation

  • Address a memory-governance bypass where revoked reads/writes were logged but the runtime result could still be published, potentially leaking revoked content or allowing it to influence published output.
  • Ensure revocation loading does not fail open by silently using an empty denial list when the store errors.

Description

  • Add enforce_memory_activity which fails publication when any memory::MemoryRejection is present, and call it before any GitHub-facing publication path so a runtime result influenced by revoked or out-of-policy memory is refused (run_and_publish).
  • Change revocation loading to propagate store errors instead of returning an empty denial list, so a storage failure surfaces and prevents assuming an empty revocation set (revocations_for(...).await.with_context(...)?).
  • Import Context from anyhow to support richer error context when revocation lookup fails.
  • Add unit tests (memory_enforcement_tests) that assert rejected memory activity blocks publishing and that accepted activity remains publishable.

Testing

  • Ran cargo check --all-targets and cargo clippy --all-targets -- -D warnings, both completed successfully.
  • Ran the targeted regression tests: cargo test -p coven-github-worker memory_enforcement_tests -- --nocapture, and the new tests passed (2 passed).
  • Ran cargo test --all where all non-worker suites passed; the worker crate overall ran many tests (75 passed, 21 failures) — the failing worker integration tests reflected shared-environment GitHub mock 403 Forbidden responses in this environment and are not regressions of the memory-enforcement logic; the targeted regression tests introduced here passed.

Codex Task

Signed-off-by: Codex <codex@openai.com>
Copilot AI lite review requested due to automatic review settings August 26, 2026 02:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the coven-github worker’s memory-governance enforcement by failing closed when the runtime reports revoked/out-of-policy memory activity, and by ensuring revocation list load failures no longer silently degrade into an “allow all” posture. This fits the adapter’s responsibility to enforce governance at the publication boundary before any GitHub-facing output is emitted.

Changes:

  • Add enforce_memory_activity to refuse publishing when any memory::MemoryRejection is present.
  • Propagate revocation-store lookup errors (with anyhow::Context) instead of defaulting to an empty denial list.
  • Add unit tests to assert rejected memory activity blocks publication while accepted activity remains publishable.
Suppressed comments (1)

crates/worker/src/lib.rs:771

  • This test asserts the full error string exactly, which is brittle if the error message is adjusted (e.g., to add counts or other safe context). It’s sufficient to assert that the failure contains the key refusal text.
        assert_eq!(
            error.to_string(),
            "runtime reported out-of-policy memory activity; refusing to publish result"
        );

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/worker/src/lib.rs
Comment on lines +840 to +845
.with_context(|| {
format!(
"failed to load memory revocations for installation {}",
task.installation_id
)
})?
Comment thread crates/worker/src/lib.rs
Comment on lines +746 to +750
anyhow::ensure!(
rejections.is_empty(),
"runtime reported out-of-policy memory activity; refusing to publish result"
);
Ok(())
Carry the runtime memory redaction fix from #86 while retaining the fail-closed publication and revocation-store enforcement in #85.
@BunsDev
BunsDev merged commit 8487d1e into main Aug 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants